Fix SIMD MinMax constant special cases by tannergooding · Pull Request #133173 · dotnet/runtime

tannergooding · 2026-09-03T15:07:23Z

SIMD Min/Max constant handling used whole-vector checks for NaN and signed zero when deciding whether native xarch Min/Max needed compensation. Constants containing a mix of ordinary and special lanes could therefore lose the managed NaN or signed-zero semantics.

This evaluates the predicates through reusable SIMD mask helpers matching IsNaN/IsNegative and AnyWhereAllBitsSet/AllWhereAllBitsSet, then uses any-lane checks for fast-path eligibility.

Fixes #133022

Note

This pull request was created with GitHub Copilot.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

azure-pipelines · 2026-09-03T15:08:20Z

Azure Pipelines: Successfully started running 5 pipeline(s). 11 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run.

dotnet-policy-service · 2026-09-03T15:08:45Z

Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch
See info in area-owners.md if you want to be subscribed.

Copilot

Copilot review overview

🔵 Needs a closer look

It changes low-level JIT SIMD constant handling and codegen-path selection, so a final human review is needed to validate cross-architecture/codegen implications despite the added regression test.

Review tier: Lite
Findings: None

What changed in this PR

This PR updates CoreCLR JIT SIMD Min/Max handling so constant-lane “special values” (NaN and signed zero) are detected per-lane (any-lane) rather than requiring whole-vector uniformity, preventing incorrect selection of an uncompensated native min/max fast-path for mixed constants. It also adds a regression test covering NaN propagation and signed-zero behavior for Vector128/Vector256 Min/Max and MinNumber/MaxNumber scenarios.

Changes:

Extend SIMD constant-evaluation helpers to support partial-vector sizes and add reusable mask predicates (IsNaN, IsNegative, Is(±)Zero, any/all mask checks).
Add GenTreeVecCon predicates for “contains any NaN / ±0 lane” and use them in the xarch Min/Max fast-path eligibility logic.
Add a new JIT regression test and wire it into the merged regression csproj.

File	Description
src/coreclr/jit/simd.h	Adds SIMD mask helper predicates and any/all mask evaluation used to reason about special lanes in constants.
src/coreclr/jit/gentree.h	Declares new `GenTreeVecCon` “contains special lane” helpers used by Min/Max handling.
src/coreclr/jit/gentree.cpp	Implements the new predicates and updates Min/Max constant-path fixup gating to use any-lane NaN/±0 detection.
src/tests/JIT/Regression/JitBlue/Runtime_133022/Runtime_133022.cs	Adds coverage for NaN propagation and signed-zero semantics across Min/Max and MinNumber/MaxNumber, including constant-vs-opaque operand cases.
src/tests/JIT/Regression/Regression_ro_2.csproj	Includes the new regression test in the merged JIT regression project.

AndyAyersMS · 2026-09-03T22:09:35Z

`MinNumber`/`MaxNumber` const-fold: this change fixes the non-AVX512 path but regresses AVX512, and the new test fails on AVX512 hardware

I ran this PR on an AVX512 box (Xeon Platinum 8370C, Windows x64, .NET 11 Checked). Summary: with DOTNET_EnableAVX512=0 the change is a complete fix; with AVX512 enabled it does not fix the original issue and introduces a new wrong-value fold.

Method. Full build.cmd clr+libs -rc Checked -lc Release at PR head f3bc6de, then a second Core_Root produced by reverting gentree.cpp / gentree.h / simd.h to the PR base 719009a and rebuilding only clr.jit. I hash-compared both Core_Root trees: clrjit.dll is the only differing file, so the JIT change is the sole variable.

1. The PR's own regression test fails on AVX512

JIT	AVX512	`Runtime_133022`
base `719009a`	on	2 failures
base `719009a`	off	2 failures
`f3bc6de`	on	2 failures
`f3bc6de`	off	0 failures

2. New wrong value under AVX512

Vector128<double>.MinNumber where the left operand is a constant containing a NaN and the right operand is opaque:

a = (0x7FF8000000000001 /* NaN  */, 0xFFF0000000000000 /* -Inf */)   constant
b = (0x7FF8000000000000 /* NaN  */, 0xFFF8000000000000 /* -NaN */)   opaque

operand delivery	base `719009a`	`f3bc6de`
both constant	`[7FF8000000000001, FFF0000000000000]`	`[7FF8000000000001, FFF0000000000000]`
left const, right opaque	`[7FF8000000000001, FFF0000000000000]`	`[7FF8000000000000, FFF8000000000000]`
left opaque, right const	`[7FF8000000000001, FFF0000000000000]`	`[7FF8000000000001, FFF0000000000000]`
both opaque (reference)	`[7FF8000000000001, FFF0000000000000]`	`[7FF8000000000001, FFF0000000000000]`

The folded result becomes the other operand wholesale. Lane 1 is the real problem and is not a NaN-payload nicety: MinNumber(-Inf, -NaN) must be -Inf, and it now yields -NaN.

Passes on 719009a at both gates and on f3bc6de with DOTNET_EnableAVX512=0; fails only on f3bc6de with AVX512 enabled. Needs a warmup loop so the method reaches tier 1.

standalone repro (exit 100 = pass, 101 = fail) — verified: PASS on base at both gates and on the PR with AVX512 off, FAIL only on the PR with AVX512 on

using System;
using System.Runtime.CompilerServices;
using System.Runtime.Intrinsics;

public static class Program
{
    [MethodImpl(MethodImplOptions.NoInlining)]
    private static double O(double v) => v;              // opaque delivery

    private static ulong Canon(double d)                 // canonicalize NaN payloads away
        => double.IsNaN(d) ? 0x7FF8000000000000UL : BitConverter.DoubleToUInt64Bits(d);

    private static ulong H(Vector128<double> v)
    {
        ulong lo = Canon(v.GetElement(0)), hi = Canon(v.GetElement(1));
        return lo ^ ((hi << 32) | (hi >> 32));
    }

    [MethodImpl(MethodImplOptions.NoInlining)]
    private static bool Agree()
    {
        // NOTE: the constants must be written inline; routing them through a local
        // or a helper defeats the fold and the test silently becomes all-opaque.
        var bothConst = Vector128.MinNumber(
            Vector128.Create(BitConverter.UInt64BitsToDouble(0x7FF8000000000001UL), BitConverter.UInt64BitsToDouble(0xFFF0000000000000UL)),
            Vector128.Create(BitConverter.UInt64BitsToDouble(0x7FF8000000000000UL), BitConverter.UInt64BitsToDouble(0xFFF8000000000000UL)));

        var rightOpaque = Vector128.MinNumber(
            Vector128.Create(BitConverter.UInt64BitsToDouble(0x7FF8000000000001UL), BitConverter.UInt64BitsToDouble(0xFFF0000000000000UL)),
            Vector128.Create(O(BitConverter.UInt64BitsToDouble(0x7FF8000000000000UL)), O(BitConverter.UInt64BitsToDouble(0xFFF8000000000000UL))));

        var leftOpaque = Vector128.MinNumber(
            Vector128.Create(O(BitConverter.UInt64BitsToDouble(0x7FF8000000000001UL)), O(BitConverter.UInt64BitsToDouble(0xFFF0000000000000UL))),
            Vector128.Create(BitConverter.UInt64BitsToDouble(0x7FF8000000000000UL), BitConverter.UInt64BitsToDouble(0xFFF8000000000000UL)));

        var reference = Vector128.MinNumber(
            Vector128.Create(O(BitConverter.UInt64BitsToDouble(0x7FF8000000000001UL)), O(BitConverter.UInt64BitsToDouble(0xFFF0000000000000UL))),
            Vector128.Create(O(BitConverter.UInt64BitsToDouble(0x7FF8000000000000UL)), O(BitConverter.UInt64BitsToDouble(0xFFF8000000000000UL))));

        ulong h = H(reference);
        return H(bothConst) == h && H(rightOpaque) == h && H(leftOpaque) == h;
    }

    public static int Main()
    {
        bool ok = true;
        for (int i = 0; i < 200_000; i++) ok = Agree();   // warm to tier 1
        Console.WriteLine(ok ? "PASS" : "FAIL: operand-delivery shapes disagree");
        return ok ? 100 : 101;
    }
}

3. At scale

A differential fuzzer that emits the same computation in all four operand-delivery shapes and compares them, 300 seeds, identical seeds on both JITs:

JIT	AVX512 on	AVX512 off
base `719009a`	274 / 300 disagree	222 / 300 disagree
`f3bc6de`	290 / 300 disagree	0 / 300

Per seed, 21 go clean -> broken and 5 broken -> clean. So the non-AVX512 result is unambiguous (222 -> 0) and AVX512 is a net regression rather than merely unimproved.

Lead, not a conclusion: the failing shape is exactly "constant operand contains a NaN, other operand opaque", which is what the new GenTreeVecCon::ContainsNaN / needsFixup predicate newly enables. I have not read the resulting codegen, so I can't say whether the bug is in the predicate or in the AVX512 lowering it now reaches.

Scope confirmed only for Vector128<double>.MinNumber under fullopts; I have not validated 256/512 or MaxNumber.

Happy to run any experiment on this hardware if it helps.

Note

This comment was generated by GitHub Copilot.

AndyAyersMS · 2026-09-03T22:35:35Z

My fuzzing indicates AVX-512 sensitivity here... can we also add some tests for it?

…the result The single constant AVX-512 Fixup escape hatch was broken three ways: the table was broadcast, so a non-uniform constant had the zero sign forced in every element rather than only the ones that needed it; the isNumber operand swap overwrote the Fixup node's own operands; and the ZERO fixup token does not distinguish +0 from -0, so it cannot produce the sign-dependent answer that MinNumber/MaxNumber require. Build the table per element so mixed constants are handled, restrict the fixup to the Min/Max cases it can actually express, and let everything else fall through to the general handling. A partially NaN constant cannot use the operand ordering either, so it falls through as well. Also prefer the general IR over the AVX-512 Range/Fixup sequence when both inputs are constant, since Range/Fixup does not constant fold. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

tannergooding · 2026-09-04T02:18:34Z

Yeah, I added some comprehensive tests covering the various edges.

I hadn't looked closely enough at the vfixupimm path and so missed that it was also only setup to handle the scalar case. Have it properly handling things now and has been tested on AVX and AVX512 for scalar and vector across the various edges.

Add general Min/Max semantic coverage for both NaN signs and a focused mixed-lane regression for the single-constant optimization. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot

Copilot review overview

🟡 Changes recommended

The updated Min/Max special-casing logic is still unreachable for the “both operands are constant vectors” scenario central to #133022, so the regression may remain unfixed.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review tier: Lite
Findings: 1 · 1

New issues introduced by this change (2)

Severity	Finding
	`src/coreclr/jit/gentree.cpp` — This block’s logic is still only reached when exactly one operand is constant (`else if ((cnsNode…
	`src/coreclr/jit/gentree.cpp` — `hasPartialNaN` is documented as tracking partially NaN constants, but the current predicate is…

tannergooding · 2026-09-04T02:55:03Z

The previous feedback was both false positives.

tannergooding · 2026-09-04T18:50:54Z

/ba-g unrelated WASM failure

tannergooding · 2026-09-04T18:51:25Z

/backport to release/11.0

tannergooding · 2026-09-04T18:51:31Z

/backport to release/10.0

github-actions · 2026-09-04T18:51:34Z

Started backporting to release/11.0 (link to workflow run)

github-actions · 2026-09-04T18:51:40Z

Started backporting to release/10.0 (link to workflow run)

github-actions · 2026-09-04T18:53:24Z

@tannergooding backporting to release/11.0 failed, the patch most likely resulted in conflicts. Please backport manually!

git am output

$ git cherry-pick a6bb62d7fcaa6ccdbb87551b48f537cd225fa1e0

Auto-merging src/coreclr/jit/gentree.cpp
Auto-merging src/coreclr/jit/gentree.h
Auto-merging src/tests/JIT/Regression/Regression_ro_2.csproj
CONFLICT (content): Merge conflict in src/tests/JIT/Regression/Regression_ro_2.csproj
error: could not apply a6bb62d7fca... Fix SIMD MinMax constant special cases (#133173)
hint: After resolving the conflicts, mark them with
hint: "git add/rm <pathspec>", then run
hint: "git cherry-pick --continue".
hint: You can instead skip this commit with "git cherry-pick --skip".
hint: To abort and get back to the state before "git cherry-pick",
hint: run "git cherry-pick --abort".
hint: Disable this message with "git config set advice.mergeConflict false"


$ git am --3way --empty=keep --ignore-whitespace --keep-non-patch changes.patch

Applying: Fix SIMD MinMax constant special cases
Using index info to reconstruct a base tree...
M	src/coreclr/jit/gentree.cpp
M	src/coreclr/jit/gentree.h
M	src/tests/JIT/Regression/Regression_ro_2.csproj
Falling back to patching base and 3-way merge...
Auto-merging src/coreclr/jit/gentree.cpp
Auto-merging src/coreclr/jit/gentree.h
Auto-merging src/tests/JIT/Regression/Regression_ro_2.csproj
CONFLICT (content): Merge conflict in src/tests/JIT/Regression/Regression_ro_2.csproj
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
hint: When you have resolved this problem, run "git am --continue".
hint: If you prefer to skip this patch, run "git am --skip" instead.
hint: To restore the original branch and stop patching, run "git am --abort".
hint: Disable this message with "git config set advice.mergeConflict false"
Patch failed at 0001 Fix SIMD MinMax constant special cases
Error: The process '/usr/bin/git' failed with exit code 128

Link to workflow output

github-actions · 2026-09-04T18:53:45Z

@tannergooding backporting to release/10.0 failed, the patch most likely resulted in conflicts. Please backport manually!

git am output

$ git cherry-pick a6bb62d7fcaa6ccdbb87551b48f537cd225fa1e0

Auto-merging src/coreclr/jit/gentree.cpp
CONFLICT (content): Merge conflict in src/coreclr/jit/gentree.cpp
Auto-merging src/coreclr/jit/gentree.h
Auto-merging src/coreclr/jit/simd.h
Auto-merging src/libraries/Common/tests/System/GenericMathTestMemberData.cs
CONFLICT (modify/delete): src/tests/JIT/Regression/Regression_ro_2.csproj deleted in HEAD and modified in a6bb62d7fca (Fix SIMD MinMax constant special cases (#133173)).  Version a6bb62d7fca (Fix SIMD MinMax constant special cases (#133173)) of src/tests/JIT/Regression/Regression_ro_2.csproj left in tree.
error: could not apply a6bb62d7fca... Fix SIMD MinMax constant special cases (#133173)
hint: After resolving the conflicts, mark them with
hint: "git add/rm <pathspec>", then run
hint: "git cherry-pick --continue".
hint: You can instead skip this commit with "git cherry-pick --skip".
hint: To abort and get back to the state before "git cherry-pick",
hint: run "git cherry-pick --abort".
hint: Disable this message with "git config set advice.mergeConflict false"


$ git am --3way --empty=keep --ignore-whitespace --keep-non-patch changes.patch

Applying: Fix SIMD MinMax constant special cases
Using index info to reconstruct a base tree...
M	src/coreclr/jit/gentree.cpp
M	src/coreclr/jit/gentree.h
M	src/coreclr/jit/simd.h
A	src/tests/JIT/Regression/Regression_ro_2.csproj
Falling back to patching base and 3-way merge...
Auto-merging src/coreclr/jit/gentree.cpp
CONFLICT (content): Merge conflict in src/coreclr/jit/gentree.cpp
Auto-merging src/coreclr/jit/gentree.h
Auto-merging src/coreclr/jit/simd.h
CONFLICT (modify/delete): src/tests/JIT/Regression/Regression_ro_2.csproj deleted in HEAD and modified in Fix SIMD MinMax constant special cases.  Version Fix SIMD MinMax constant special cases of src/tests/JIT/Regression/Regression_ro_2.csproj left in tree.
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
hint: When you have resolved this problem, run "git am --continue".
hint: If you prefer to skip this patch, run "git am --skip" instead.
hint: To restore the original branch and stop patching, run "git am --abort".
hint: Disable this message with "git config set advice.mergeConflict false"
Patch failed at 0001 Fix SIMD MinMax constant special cases
Error: The process '/usr/bin/git' failed with exit code 128

Link to workflow output

Fix SIMD MinMax constant special cases

f3bc6de

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot AI lite review requested due to automatic review settings September 3, 2026 15:07

dotnet-policy-service Bot assigned tannergooding Sep 3, 2026

github-actions Bot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Sep 3, 2026

Copilot started reviewing on behalf of tannergooding September 3, 2026 15:08 View session

tannergooding requested a review from AndyAyersMS September 3, 2026 15:10

Copilot AI reviewed Sep 3, 2026

View reviewed changes

build-analysis Bot mentioned this pull request Sep 3, 2026

Test Failure: System.Net.WebSockets.Client.Tests.CancelTest_SharedHandler_Loopback.ConnectAsync_Cancel_ThrowsCancellationException #130439

Open

Cover signed NaNs in vector min/max tests

8dcd25b

Add general Min/Max semantic coverage for both NaN signs and a focused mixed-lane regression for the single-constant optimization. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot AI review requested due to automatic review settings September 4, 2026 02:33

Copilot started reviewing on behalf of tannergooding September 4, 2026 02:33 View session

Copilot AI reviewed Sep 4, 2026

View reviewed changes

Comment thread src/coreclr/jit/gentree.cpp

Comment thread src/coreclr/jit/gentree.cpp

AndyAyersMS approved these changes Sep 4, 2026

View reviewed changes

tannergooding merged commit a6bb62d into dotnet:main Sep 4, 2026
161 of 163 checks passed

tannergooding deleted the tannergooding-fix-jit-constant-special-cases branch September 4, 2026 18:51

This was referenced Sep 4, 2026

[release/11.0] Fix SIMD MinMax constant special cases #133256

Open

[release/10.0] Fix SIMD MinMax constant special cases #133257

Open

Uh oh!

Conversation

tannergooding commented Sep 3, 2026

Uh oh!

azure-pipelines Bot commented Sep 3, 2026

Uh oh!

dotnet-policy-service Bot commented Sep 3, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🔵 Needs a closer look

Uh oh!

AndyAyersMS commented Sep 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

MinNumber/MaxNumber const-fold: this change fixes the non-AVX512 path but regresses AVX512, and the new test fails on AVX512 hardware

1. The PR's own regression test fails on AVX512

2. New wrong value under AVX512

3. At scale

Uh oh!

AndyAyersMS commented Sep 3, 2026

Uh oh!

tannergooding commented Sep 4, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

tannergooding commented Sep 4, 2026

Uh oh!

tannergooding commented Sep 4, 2026

Uh oh!

Uh oh!

tannergooding commented Sep 4, 2026

Uh oh!

tannergooding commented Sep 4, 2026

Uh oh!

github-actions Bot commented Sep 4, 2026

Uh oh!

github-actions Bot commented Sep 4, 2026

Uh oh!

github-actions Bot commented Sep 4, 2026

Uh oh!

github-actions Bot commented Sep 4, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

AndyAyersMS commented Sep 3, 2026 •

edited

Loading

`MinNumber`/`MaxNumber` const-fold: this change fixes the non-AVX512 path but regresses AVX512, and the new test fails on AVX512 hardware